Skip to content

Conversation

@amyfromandi
Copy link
Collaborator

Merging the database schema setup for the saved_locations user features

Copy link
Member

@davenquinn davenquinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally this looks good but there are some changes that might make a stronger schema design.

longitude numeric not null,
created_at timestamp default now() not null,
updated_at timestamp default now() not null,
category saved_locations_enum
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be an enum or a link to a related table? the nice thing about the latter is that it is easier to update and can have "details" (e.g., an explanation of what the category means, a symbol ID, etc.) attached.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great question. I think having set categories as an enum type so that the user can quickly select...and then have a separate tags table so that the user can define and search by their locations using their own tag definitions. I'll add this to the schema to get your thoughts.

alter table saved_locations
owner to "macrostrat-admin";

grant delete, insert, select, update on saved_locations to web_anon;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want anonymous users to be able to delete and insert locations? We could lock this down a bit more even without implementing row-level security...

Copy link
Collaborator Author

@amyfromandi amyfromandi Feb 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thoughts were to only give CRUD ops only to authorized users and not web_anon. Does web_anon require a login to access macrostrat? If not, I would assume they wouldn't get access to the saved_locations feature at all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep agreed

location_name varchar(120) not null,
location_description text,
latitude numeric not null,
longitude numeric not null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How will you decide how far to zoom in for specific saved locations? Is this something that needs to be saved in the database?

  • Maybe we could add a view_params JSON column to house view parameters in a less structured way?
  • We could alternatively/additionally set a radius value that would define whether the location was small or large

The Zoom level/radius is mostly only a problem because we can add locations from a very zoomed out perspective, so maybe the solution would be to just disallow that in the UI.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've stored this as an additional text column for now. I'm going to look into the how we can take these extra params, transform them into json and import them into the db.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having optional radius (number in meters) and camera_position (JSON column with format matching https://github.com/UW-Macrostrat/web-components/blob/3f7eb7aa9e1c38dca0d2689d9a127a67a2cd7e05/packages/mapbox-utils/src/position.ts#L13) would do nicely.

If we store the elevation of the point, we can infer the camera position from pitch and bearing also, so maybe that's better.

@davenquinn
Copy link
Member

  • We could go ahead and attempt to integrate row-level security here, or save it for another pull request
  • It would be great to package this as a migration, but that can also be another pull request

Copy link
Member

@davenquinn davenquinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Location information updates

references macrostrat_auth."user",
location_name varchar(120) not null,
location_description text,
latitude numeric not null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Latitude and longitude should be changed to a point geometry with SRID 4326. Geometry(Point, 4326)

@amyfromandi amyfromandi requested a review from davenquinn March 24, 2025 17:35
@amyfromandi
Copy link
Collaborator Author

amyfromandi commented Mar 24, 2025

Saved locations feature is added to the migrations code here

@amyfromandi amyfromandi merged commit cf24572 into main Mar 24, 2025
@amyfromandi amyfromandi deleted the saved_locations branch March 24, 2025 17:40
@amyfromandi
Copy link
Collaborator Author

@davenquinn The user_locations migration now successfully runs. Additionally, all CRUD operations work via the PostgREST API using web_anon. We are ready for the front-end development, unless we need any custom v3 endpoints created beyond the ones listed below. We'll need to address authentication and RLS for particular users as well.

Migrations:
- baseline:               already applied
- macrostrat-api:         already applied
**- user-saved-locations:   should be applied**
- integrations-tables:    already applied
- points:                 already applied
- macrostrat-mariadb:     already applied
- map-source-slug:        already applied
- ingest-state-type:      already applied
- maps-scale-type:        already applied
- maps-lines-oriented:    already applied
- api-v3:                 already applied
- macrostrat-tileserver:  already applied
- maps-sources-api:        always applied
- maps-source-operations: already applied
- storage-scheme:         already applied
- ingest-metadata:        already applied
- macrostrat-core-v2:     already applied
- macrostrat-tile-layers:  always applied
- map-ingestion-api:      already applied
- partition-maps:         already applied
- column-builder:         already applied
- partition-carto:        already applied
- maps-sources:           already applied

Applied 3 migrations in 165.18 seconds
Completed: ['baseline', 'macrostrat-api', 'integrations-tables',
'points', 'macrostrat-mariadb', 'map-source-slug',
'ingest-state-type', 'maps-scale-type', 'maps-lines-oriented',
'api-v3', 'macrostrat-tileserver', 'maps-source-operations',
'storage-scheme', 'ingest-metadata', 'macrostrat-core-v2',
'map-ingestion-api', 'partition-maps', 'column-builder',
'partition-carto', 'maps-sources', **'user-saved-locations'**,
'maps-sources-api', 'macrostrat-tile-layers']

These are the PostGREST URL examples that work:
POST https://dev2.macrostrat.org/api/pg/user_locations
example json:

{
  "user_id": 46,
  "name": "Volcanic Cone Rim",
  "description": "Overlook point on rim of extinct volcanic cone.",
  "point": "SRID=4326;POINT(-155.292 19.406)",
  "zoom": 15.0,
  "meters_from_point": 80.0,
  "elevation": 2395.0,
  "azimuth": 30.0,
  "pitch": -5.0,
  "map_layers": ["satellite", "geology", "elevation"]
}

GET https://dev2.macrostrat.org/api/pg/user_locations
PATCH https://dev2.macrostrat.org/api/pg/user_locations?id=eq.{{id to update here}}
PUT https://dev2.macrostrat.org/api/pg/user_locations?id=eq.{{id to update here}}
DELETE https://dev2.macrostrat.org/api/pg/user_locations?id=eq.{{id to delete here}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants